Simplify semantics of private fields; add self syntax for classes by rossberg · Pull Request #79 · caffeinelabs/motoko

rossberg · 2018-11-22T12:16:38Z

Change semantics of private specifier such that it is per-instance. This simplifies its semantics to essentially a scoping restriction. It also is the only semantics that actually is implementable for actors.

At the same time, add syntax for binding "this" to classes. This was necessary to be able to rewrite some of the examples previously using per-class private.

… to class

nomeata · 2018-11-22T14:37:51Z

Sounds good to me what you describe, and also the changes to compile.ml. It looks like you deleted some test output that (wasm-run.ok files) that are actually needed, so Jenkins is unhappy.

rossberg · 2018-11-22T14:59:51Z

Hm, seems like I don't have access rights to see the Jenkins failures. But the only .ok files I removed are the ones for the dot-private test, which I also removed (because the feature it tests no longer exists). Do I have to adjust something else?

crusso · 2018-11-22T15:03:55Z

I'm fine with this. I also don't have access to the Jenkins output. Are the tests running ok locally?

rossberg · 2018-11-22T15:07:19Z

Yes, I don't see any local failures.

crusso · 2018-11-22T15:15:00Z

Did you run the test in test\run-dfinity too? I don't think make in test does that.

rossberg · 2018-11-22T15:27:07Z

The only DFN failures I see are the known ones using Nop, because Hero can't handle that yet.

crusso · 2018-11-22T15:31:25Z

https://github.com/dfinity-lab/ops-in-nix/blob/master/VPN.md are the instructions to get access, but with Thanksgiving on your best may be Maciej

nomeata · 2018-11-23T07:45:15Z

I thought i removed the use of Nop from our master. Is your branch up-to-date?

nomeata · 2018-11-23T08:07:40Z

Also, the test output files that your pull request removes (see the diff) are from run/, so this is independent of dvm. Can't tell from afar what went wrong.

Does running make accept in run/ add or change any files?

nomeata · 2018-11-23T08:10:26Z

But the only .ok files I removed are the ones for the dot-private test, which I also removed (because the feature it tests no longer exists).

I don't think you remove any tests. Check the diff!

rossberg · 2018-11-26T11:01:02Z

@nomeata, I just tried rebasing on latest master, still seeing the nop errors.

test/run/this-dot-private.as got removed in this PR, see here.

rossberg · 2018-11-26T11:02:15Z

@crusso, funny enough, I can't access those instructions either.

rossberg · 2018-11-26T11:04:05Z

Oh, I see, document was moved.

paulyoung · 2018-11-26T18:47:54Z

Some minor feedback after using these changes is that once I started using the built-in self syntax I had to use like pretty pervasively. I was expecting this based on the diff here and didn't think it was so bad until I had to start using it in return values as well.

In places where I had intended to use the built-in self syntax I'm currently doing this type of thing instead:

class Foo<A>(value_ : A) {
  value = value_;
  private self = Foo<A>(value);

  bar = func () {
    baz<A>(self);
  };
};

let baz = func <A>(foo : Foo<A>) {};

My sense is that this might not be a good idea (maybe semantically it's fine, and the overhead is minimal?) but either way I'd like to better understand the implications of that.

If it turns out to be fine, could the built in self syntax be sugar for what I'm doing above to avoid the need to use like?

paulyoung · 2018-11-26T18:53:01Z

Contrived example, based on the above.

class Foo<A>(value_ : A) {
  value = value_;
  private self = Foo<A>(value);

  bar = func () : Foo<A> {
    baz<A>(self);
  };
};

/* Basically the identity function */
let baz = func <A>(foo : Foo<A>) : Foo<A> {
  foo;
};

vs

class Foo<A>(value_ : A) = self {
  value = value_;

  bar = func () : like Foo<A> {
    baz<A>(self);
  };
};

/* Basically the identity function */
let baz = func <A>(foo : like Foo<A>) : like Foo<A> {
  foo;
};

paulyoung · 2018-11-26T19:17:48Z

Actually, that's not even what I'm doing because self would get out of sync.

I'm making a local "self" at function call sites like this:

class Foo<A>(value_ : A) {
  value = value_;

  bar = func () {
    let self = Foo<A>(value);
    baz<A>(self);
  };
};

let baz = func <A>(foo : Foo<A>) {};

## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@eef711d1...91832636](dfinity/ic-hs@eef711d...9183263) * [`4920ec29`](dfinity/ic-hs@4920ec2) System API for ECDSA signing ([dfinity/ic-hs⁠#79](https://github.com/dfinity/ic-hs/issues/79)) * [`47b7f0b6`](dfinity/ic-hs@47b7f0b) Mirror changes of the spec related to the canister HTTP calls ([dfinity/ic-hs⁠#85](https://github.com/dfinity/ic-hs/issues/85)) * [`91832636`](dfinity/ic-hs@9183263) upgrade nixpkgs to release-22.05 ([dfinity/ic-hs⁠#76](https://github.com/dfinity/ic-hs/issues/76))

## Changelog for ic-wasm: Branch: main Commits: [dfinity/ic-wasm@48db35ee...bdc074a8](dfinity/ic-wasm@48db35e...bdc074a) * [`bdc074a8`](dfinity/ic-wasm@bdc074a) fix: limit_resource works with wasm64 ([dfinity/ic-wasm⁠#79](https://github.com/dfinity/ic-wasm/issues/79))

## Changelog for ic-wasm: Branch: main Commits: [dfinity/ic-wasm@48db35ee...72d2bc28](dfinity/ic-wasm@48db35e...72d2bc2) * [`bdc074a8`](dfinity/ic-wasm@bdc074a) fix: limit_resource works with wasm64 ([dfinity/ic-wasm⁠#79](https://github.com/dfinity/ic-wasm/issues/79)) * [`72d2bc28`](dfinity/ic-wasm@72d2bc2) Validate the manipulated module before emitting it and give a warning if that fails. ([dfinity/ic-wasm⁠#80](https://github.com/dfinity/ic-wasm/issues/80))

## Changelog for ic-wasm: Branch: main Commits: [dfinity/ic-wasm@48db35ee...707016c2](dfinity/ic-wasm@48db35e...707016c) * [`bdc074a8`](dfinity/ic-wasm@bdc074a) fix: limit_resource works with wasm64 ([dfinity/ic-wasm⁠#79](https://github.com/dfinity/ic-wasm/issues/79)) * [`72d2bc28`](dfinity/ic-wasm@72d2bc2) Validate the manipulated module before emitting it and give a warning if that fails. ([dfinity/ic-wasm⁠#80](https://github.com/dfinity/ic-wasm/issues/80)) * [`707016c2`](dfinity/ic-wasm@707016c) Build musl target with docker alpine in release workflow ([dfinity/ic-wasm⁠#1](https://github.com/dfinity/ic-wasm/issues/1)) ([dfinity/ic-wasm⁠#82](https://github.com/dfinity/ic-wasm/issues/82))

## Changelog for ic-wasm: Branch: main Commits: [dfinity/ic-wasm@48db35ee...41bb2f59](dfinity/ic-wasm@48db35e...41bb2f5) * [`bdc074a8`](dfinity/ic-wasm@bdc074a) fix: limit_resource works with wasm64 ([dfinity/ic-wasm⁠#79](https://github.com/dfinity/ic-wasm/issues/79)) * [`72d2bc28`](dfinity/ic-wasm@72d2bc2) Validate the manipulated module before emitting it and give a warning if that fails. ([dfinity/ic-wasm⁠#80](https://github.com/dfinity/ic-wasm/issues/80)) * [`707016c2`](dfinity/ic-wasm@707016c) Build musl target with docker alpine in release workflow ([dfinity/ic-wasm⁠#1](https://github.com/dfinity/ic-wasm/issues/1)) ([dfinity/ic-wasm⁠#82](https://github.com/dfinity/ic-wasm/issues/82)) * [`41bb2f59`](dfinity/ic-wasm@41bb2f5) chore: Allow sign_with_schnorr ([dfinity/ic-wasm⁠#83](https://github.com/dfinity/ic-wasm/issues/83))

## Changelog for ic-wasm: Branch: main Commits: [dfinity/ic-wasm@48db35ee...4c52e75c](dfinity/ic-wasm@48db35e...4c52e75) * [`bdc074a8`](dfinity/ic-wasm@bdc074a) fix: limit_resource works with wasm64 ([dfinity/ic-wasm⁠#79](https://github.com/dfinity/ic-wasm/issues/79)) * [`72d2bc28`](dfinity/ic-wasm@72d2bc2) Validate the manipulated module before emitting it and give a warning if that fails. ([dfinity/ic-wasm⁠#80](https://github.com/dfinity/ic-wasm/issues/80)) * [`707016c2`](dfinity/ic-wasm@707016c) Build musl target with docker alpine in release workflow ([dfinity/ic-wasm⁠#1](https://github.com/dfinity/ic-wasm/issues/1)) ([dfinity/ic-wasm⁠#82](https://github.com/dfinity/ic-wasm/issues/82)) * [`41bb2f59`](dfinity/ic-wasm@41bb2f5) chore: Allow sign_with_schnorr ([dfinity/ic-wasm⁠#83](https://github.com/dfinity/ic-wasm/issues/83)) * [`4c52e75c`](dfinity/ic-wasm@4c52e75) fix: Feature wasm-opt ([dfinity/ic-wasm⁠#84](https://github.com/dfinity/ic-wasm/issues/84))

Simplify semantics of private fields to per-instance; add self syntax…

f1eec8b

… to class

rossberg requested review from crusso and nomeata November 22, 2018 12:16

crusso and others added 2 commits November 26, 2018 16:15

merged with master

8aa129c

Re-add some wasm-run.ok files that got lost

8c0f55e

crusso merged commit 40291dc into master Nov 26, 2018

paulyoung mentioned this pull request Nov 26, 2018

Implement uniformity check for type recursion #76

Closed

nomeata deleted the private branch November 26, 2018 21:15

Conversation

rossberg commented Nov 22, 2018

Uh oh!

nomeata commented Nov 22, 2018

Uh oh!

rossberg commented Nov 22, 2018

Uh oh!

crusso commented Nov 22, 2018

Uh oh!

rossberg commented Nov 22, 2018

Uh oh!

crusso commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rossberg commented Nov 22, 2018

Uh oh!

crusso commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomeata commented Nov 23, 2018

Uh oh!

nomeata commented Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomeata commented Nov 23, 2018

Uh oh!

rossberg commented Nov 26, 2018

Uh oh!

rossberg commented Nov 26, 2018

Uh oh!

rossberg commented Nov 26, 2018

Uh oh!

paulyoung commented Nov 26, 2018

Uh oh!

paulyoung commented Nov 26, 2018

Uh oh!

paulyoung commented Nov 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

crusso commented Nov 22, 2018 •

edited

Loading

crusso commented Nov 22, 2018 •

edited

Loading

nomeata commented Nov 23, 2018 •

edited

Loading